Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated app to rails 7 #3426

Open
wants to merge 18 commits into
base: development
Choose a base branch
from
Open

Updated app to rails 7 #3426

wants to merge 18 commits into from

Conversation

benjaminfaure
Copy link
Contributor

Updated app to Rails 7.1

Copy link

github-actions bot commented Jun 14, 2024

</tr>
1 Error
🚫

Please include a CHANGELOG entry.

You can find it at [CHANGELOG.md](https://github.com/DMPRoadmap/roadmap/blob/main/CHANGELOG.md).
1 Warning
⚠️ This PR is too big! Consider breaking it down into smaller PRs.

Generated by 🚫 Danger

Copy link
Contributor

@aaronskiba aaronskiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error shows a change in the behaviour of Template.visibility when upgrading to Rails 7:

Failures:

  1) Template#upgrade_customization! sets the visibility to Organisationally visible
     Failure/Error: expect(subject.visibility).to eql(Template.visibilities['organisationally_visible'])
     
       expected: 0
            got: "organisationally_visible"
     
       (compared using eql?)
     # ./spec/models/template_spec.rb:1155:in `block (3 levels) in <main>'
     # /usr/share/rvm/gems/ruby-3.0.5@upstream/gems/webmock-3.23.1/lib/webmock/rspec.rb:39:in `block (2 levels) in <main>'

Rails 6

aaron@ubuntu:~/Documents/GitHub/roadmap_upstream
$ rails c
Running via Spring preloader in process 1979938
Loading development environment (Rails 6.1.7.7)
3.0.5 :001 > Template.first.visibility
Creating scope :publicly_visible. Overwriting existing method Template.publicly_visible.
Creating scope :organisationally_visible. Overwriting existing method Template.organisationally_visible.
  Template Load (1.6ms)  SELECT "templates".* FROM "templates" ORDER BY "templates"."id" ASC LIMIT $1  [["LIMIT", 1]]
 => 0 

Rails 7

aaron@ubuntu:~/Documents/GitHub/roadmap_upstream
$ rails c
Running via Spring preloader in process 1983441
Loading development environment (Rails 7.1.3.4)
3.0.5 :001 > Template.first.visibility
  Template Load (1.5ms)  SELECT "templates".* FROM "templates" ORDER BY "templates"."id" ASC LIMIT $1  [["LIMIT", 1]]
 => "organisationally_visible"

app/models/org.rb Outdated Show resolved Hide resolved
Addresses the following error that was encountered when attempting to execute the GitHub Actions that correspond with the edited workflows files:

https://github.com/DMPRoadmap/roadmap/actions/runs/9513143079/job/26222602325
3s

```
Run bundle exec rails db:create RAILS_ENV=test
To use retry middleware with Faraday v2.0+, install `faraday-retry` gem
To use multipart middleware with Faraday v2.0+, install `faraday-multipart` gem; note: this is used by the ManageGHES client for uploading licenses
Copying Bootstrap glyphicons to the public directory ...
Copying TinyMCE skins to the public directory ...
/home/runner/work/roadmap/roadmap/config/initializers/recaptcha.rb:8:in `block in <main>': undefined method `[]' for nil:NilClass (NoMethodError)
	from /home/runner/work/roadmap/roadmap/vendor/bundle/ruby/3.0.0/gems/recaptcha-5.17.0/lib/recaptcha.rb:37:in `configure'
	from /home/runner/work/roadmap/roadmap/config/initializers/recaptcha.rb:7:in `<main>'
```
Prior to this commit, the Rails credentials were not being updated during this setup process.
This commit undoes some Rubocop fixes made from a prior commit ( bda5b6e ). However, it also resolves the following error that was being raised: https://github.com/DMPRoadmap/roadmap/actions/runs/9845084297/job/27179836711
aaronskiba and others added 9 commits July 9, 2024 14:19
Omitting the arguments results in lambda implicitly using self, which appears to be the desired behaviour here. It also resolves the Rubocop offences.
`template.visibilty` now returns a string rather than an integer.

The Rails 7 upgrade actually fixes a couple of bugs within `app/views/org_admin/templates/_form.html.erb` and `app/views/org_admin/templates/_show.html.erb`. Prior to this upgrade, template.visibility would return an integer. Now that it is returning a string, the `f.object.visibility == 'organisationally_visible'` and `template.visibility == 'organisationally_visible'` checks within the aforementioned files are behaving as desired.
Prior to this commit, the default checked/unchecked values were used (i.e. "1" would be returned when checked, and "0" would be returned when unchecked). However, the box is meant to be checked when selecting 'organisationally_visible' ('for internal %{org_name} use only'), which makes the default checked/unchecked values opposite to the mapping of our enums (i.e. `{"organisationally_visible"=>0, "publicly_visible"=>1}`).
Rails 7 appears to apply stricter parsing rules. If the Content-Type is not JSON, then the body will not be parsed as JSON.
Prior to this code change, any value assigned to the `'data-method':` attribute of the `link_to` method was not being read (and instead defaulting to `GET`). This was resulting in the breaking of several `spec/features/` tests (https://github.com/DMPRoadmap/roadmap/actions/runs/9946998559/job/27478725801). The `@rails/ujs` library is meant to handle this `'data-method':` attribute.
Fix PostgreSQL GitHub Action and Tests For Rails 7 Upgrade
Added `coder:` and `type:` keywords in various places to address deprecation warnings.

Example warning (before adding `type: ` keyword in `app/models/user.rb`:
````
Please pass the class as a keyword argument:

  serialize :prefs, type: Hash
 (called from <class:User> at /path/to/app/models/user.rb:73)
DEPRECATION WARNING: Passing the class as positional argument is deprecated and will be removed in Rails 7.2.
```
This change addresses the following deprecation warnings:
```
DEPRECATION WARNING: Setting action_dispatch.show_exceptions to false is deprecated. Set to :none instead. (called from rescue in call at /usr/share/rvm/gems/ruby-3.0.5@upstream/gems/actionpack-7.1.3.4/lib/action_dispatch/middleware/debug_exceptions.rb:43)
DEPRECATION WARNING: Setting action_dispatch.show_exceptions to false is deprecated. Set to :none instead. (called from rescue in call at /usr/share/rvm/gems/ruby-3.0.5@upstream/gems/actionpack-7.1.3.4/lib/action_dispatch/middleware/show_exceptions.rb:36)
```
@aaronskiba aaronskiba mentioned this pull request Sep 4, 2024
@aaronskiba
Copy link
Contributor

Regarding some of the failing checks:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants